Skip to content

fix: add type tests#65

Merged
nzakas merged 1 commit intomainfrom
test-types
Nov 21, 2024
Merged

fix: add type tests#65
nzakas merged 1 commit intomainfrom
test-types

Conversation

@fasttime
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request?

Add type tests and fix some problems in the types.

What changes did you make? (Give an overview)

Added type tests to the CI workflow. I had to fix some minor issues in the typings of rules metadata and in the recommended config to match the signature of ESLint.Plugin. This is why this pull request is tagged fix:.

Related Issues

fixes #61

Is there anything you'd like reviewers to focus on?

@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label Nov 21, 2024
Comment on lines -38 to +42
rules: {
rules: /** @type {const} */ ({
"json/no-duplicate-keys": "error",
"json/no-empty-keys": "error",
"json/no-unsafe-values": "error",
},
}),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is const cast syntax. I'm using this to ensure that properties in the rules object have type "error" rather than string, because string is too wide to be assigned to a severity:

    type StringSeverity = "off" | "warn" | "error";

https://github.com/eslint/eslint/blob/v9.15.0/lib/types/index.d.ts#L917

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat trick!

export default {
meta: {
type: "problem",
type: /** @type {const} */ ("problem"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that the type property has type "problem" rather than string so it matches the definition in the interface:

        type?: "problem" | "suggestion" | "layout" | undefined;

https://github.com/eslint/eslint/blob/v9.15.0/lib/types/index.d.ts#L760

@fasttime fasttime marked this pull request as ready for review November 21, 2024 09:34
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This is really cool. Thanks!

@nzakas nzakas merged commit a6c0bc9 into main Nov 21, 2024
@nzakas nzakas deleted the test-types branch November 21, 2024 16:42
@mdjermanovic mdjermanovic mentioned this pull request Nov 22, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change Request: add type tests

2 participants